Summary

• extract storage file-path helper functions out of lib/storage.ts into a dedicated helper module
• keep the existing backup, wal, reset-marker, and flagged-path behavior intact while continuing the storage split in small slices

What Changed

• added lib/storage/file-paths.ts with the backup-path, wal-path, reset-marker, and flagged-storage path helpers
• updated lib/storage.ts to import the extracted helpers and keep the public getFlaggedAccountsPath() surface intact
• preserved the existing storage behavior and coverage through the storage test suite

Validation

• npm run test -- test/storage.test.ts
• npm run lint
• npm run typecheck
• npm run build

Risk and Rollback

• Risk level: low
• Rollback plan: revert 21aaa89 to restore the inline storage file-path helper implementation

Additional Notes

• this continues the storage split after the error-hint and identity-helper extractions in the same isolated worktree and stacked review flow

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this pr extracts the storage file-path helper functions (getAccountsBackupPath, getAccountsWalPath, getIntentionalResetMarkerPath, getAccountsBackupRecoveryCandidates, getFlaggedAccountsPath) and the three suffix constants (ACCOUNTS_BACKUP_SUFFIX, ACCOUNTS_WAL_SUFFIX, RESET_MARKER_SUFFIX) out of lib/storage.ts into the new lib/storage/file-paths.ts module, continuing the incremental storage split. the previously flagged duplicate-constants risk is resolved — storage.ts now imports from file-paths.ts and the local copies are deleted, making file-paths.ts the single source of truth.

• lib/storage/file-paths.ts: new dedicated helper module; all path and suffix constants exported from here; ACCOUNTS_BACKUP_HISTORY_DEPTH correctly kept private
• lib/storage.ts: local constant and function definitions removed; public getFlaggedAccountsPath() surface preserved via a thin wrapper that delegates to the extracted helper
• test/storage-file-paths.test.ts: new vitest suite covering posix paths, backup candidate ordering, and flagged-path construction; windows-path test is tautological on linux ci (both implementation and assertion use the same node:path call) — consider using path.win32 for a meaningful cross-platform check

Confidence Score: 5/5

• safe to merge — clean refactor with no behavior change and the previous duplicate-constants concern fully resolved
• the duplicate-constants issue from the prior review is fully addressed; file-paths.ts is the sole source of truth for suffixes and path helpers; storage.ts public api is intact; the only remaining item is a non-blocking p2 on a tautological windows-path test that doesn't affect runtime correctness
• no files require special attention

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["lib/storage.ts\n(public API)"]
    B["lib/storage/file-paths.ts\n(new helper module)"]
    C["ACCOUNTS_BACKUP_SUFFIX\nACCOUNTS_WAL_SUFFIX\nRESET_MARKER_SUFFIX\n(single source of truth)"]
    D["getAccountsBackupPath()\ngetAccountsWalPath()\ngetIntentionalResetMarkerPath()\ngetAccountsBackupRecoveryCandidates()\ngetFlaggedAccountsPath()"]
    E["storage.ts callers:\ngetAccountsBackupRecoveryCandidatesWithDiscovery()\nisRotatingBackupTempArtifact()\ngetFlaggedAccountsPath()\ngetLegacyFlaggedAccountsPath()"]

    A -->|"imports (named re-export)"| B
    B --> C
    B --> D
    A --> E
    E -->|"uses imported helpers + constants"| B

This is a comment left during a code review.
Path: test/storage-file-paths.test.ts
Line: 48-56

Comment:
**tautological windows-path test**

the assertion mirrors the implementation identically — both sides call `join(dirname(storagePath), fileName)` via the same `node:path` import. on linux ci, `dirname('C:\\Users\\user\\.codex\\openai-codex-accounts.json')` returns `'.'` (backslashes aren't posix separators), so both sides produce `'openai-codex-blocked-accounts.json'` — a relative path — and the test passes vacuously without verifying correct windows sibling-path resolution.

to test windows semantics cross-platform, assert against a hard-coded expected value computed with `path.win32`:

```ts
import { join as joinWin32, dirname as dirnameWin32 } from "node:path/win32";

it("builds sibling paths for windows-style storage paths", () => {
  const storagePath = String.raw`C:\Users\user\.codex\openai-codex-accounts.json`;
  const fileName = "openai-codex-blocked-accounts.json";

  // use path.win32 explicitly so the assertion is meaningful on posix ci too
  expect(getFlaggedAccountsPath(storagePath, fileName)).toBe(
    joinWin32(dirnameWin32(storagePath), fileName),
  );
});
```

note: this also means the implementation in `file-paths.ts` should use `path.win32` (or `path.normalize`) for windows paths if cross-platform correctness is required on the caller side.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (3): Last reviewed commit: "test: cover storage file path helpers" | Re-trigger Greptile}